src/Diwink.Extensions.EntityFrameworkCore.Tests.Integration/Diwink.Extensions.EntityFrameworkCore.Tests.Integration.csproj (1)

12-15: Pin floating package versions to keep CI restores deterministic.

Lines 12, 13, and 15 use wildcard versions (8.*, 10.*, 4.*), which can resolve differently over time and make integration test behavior drift between runs.

Proposed change

-    <PackageReference Include="FluentAssertions" Version="8.*" />
-    <PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="10.*" />
+    <PackageReference Include="FluentAssertions" Version="8.9.0" />
+    <PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="10.0.5" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.14.1" />
-    <PackageReference Include="Testcontainers.MsSql" Version="4.*" />
+    <PackageReference Include="Testcontainers.MsSql" Version="4.0.0" />

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore.Tests.Integration/Diwink.Extensions.EntityFrameworkCore.Tests.Integration.csproj`
around lines 12 - 15, Replace the floating wildcard package versions with exact,
pinned versions to ensure deterministic CI restores: update the PackageReference
entries for FluentAssertions, Microsoft.EntityFrameworkCore.SqlServer, and
Testcontainers.MsSql (the elements with Include="FluentAssertions",
Include="Microsoft.EntityFrameworkCore.SqlServer",
Include="Testcontainers.MsSql") to use explicit version numbers instead of
"8.*", "10.*", and "4.*" respectively (leave Microsoft.NET.Test.Sdk as-is if
already pinned); commit the csproj change so CI always restores the specified
exact package versions.

src/Diwink.Extensions.EntityFrameworkCore/Exceptions/UnsupportedNavigationMutatedException.cs (1)

12-23: Validate relationshipType before storing it.

Line 22 sets a contract property from unvalidated input; enforce non-empty values to keep error payloads dependable.

Proposed fix

     public UnsupportedNavigationMutatedException(
         string relationshipPath,
         string relationshipType)
         : base(
             $"Mutation detected in unsupported navigation '{relationshipPath}' " +
             $"(relationship type: {relationshipType}). This relationship type is not " +
             "in the v2 supported contract. The entire operation was rejected. " +
             "Unchanged unsupported navigations would have been silently skipped.",
             relationshipPath)
     {
+        ArgumentException.ThrowIfNullOrWhiteSpace(relationshipType);
         RelationshipType = relationshipType;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore/Exceptions/UnsupportedNavigationMutatedException.cs`
around lines 12 - 23, The UnsupportedNavigationMutatedException constructor
stores the unvalidated parameter relationshipType into the RelationshipType
property; validate that relationshipType is not null/empty (e.g., throw
ArgumentNullException or ArgumentException after trimming) before assigning it.
Update the UnsupportedNavigationMutatedException(string relationshipPath, string
relationshipType) constructor to check relationshipType (and relationshipPath if
desired) and throw an appropriate exception for invalid values, then assign
RelationshipType only after validation so error payloads remain dependable.

src/Diwink.Extensions.EntityFrameworkCore/GraphUpdate/OperationGuard.cs (1)

16-16: Return an immutable view for Errors.

Line 16 exposes the backing List<GraphUpdateException> instance. Internal callers can cast back to List<T> and mutate state outside AddError.

Proposed fix

-    public IReadOnlyList<GraphUpdateException> Errors => _errors;
+    public IReadOnlyList<GraphUpdateException> Errors => _errors.AsReadOnly();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Diwink.Extensions.EntityFrameworkCore/GraphUpdate/OperationGuard.cs` at
line 16, The Errors property currently returns the mutable backing _errors list
directly; change the getter to return an immutable view (e.g. wrap _errors with
ReadOnlyCollection via _errors.AsReadOnly() or return a ReadOnlyCollection
cached field) so callers cannot cast back to List<T> and mutate state; keep
mutation confined to AddError and reference GraphUpdateException and
_errors/Errors/AddError to locate the change.

src/Diwink.Extensions.EntityFrameworkCore/Exceptions/AmbiguousOwnershipSemanticsException.cs (1)

11-20: Validate missingDetail to preserve exception quality.

MissingDetail is a public diagnostic field; Lines 11–20 should reject null/whitespace input.

Proposed fix

     public AmbiguousOwnershipSemanticsException(
         string relationshipPath,
         string missingDetail)
         : base(
             $"Ambiguous ownership semantics at '{relationshipPath}': {missingDetail}. " +
             "The contract requires explicit requiredness/ownership metadata.",
             relationshipPath)
     {
+        ArgumentException.ThrowIfNullOrWhiteSpace(missingDetail);
         MissingDetail = missingDetail;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore/Exceptions/AmbiguousOwnershipSemanticsException.cs`
around lines 11 - 20, The constructor AmbiguousOwnershipSemanticsException
should validate the missingDetail parameter and reject null/whitespace to
preserve exception integrity; inside the
AmbiguousOwnershipSemanticsException(string relationshipPath, string
missingDetail) constructor, check string.IsNullOrWhiteSpace(missingDetail) and
throw an ArgumentException or ArgumentNullException with a clear message
(including the parameter name "missingDetail") before calling base, ensuring the
public MissingDetail property is only assigned valid non-empty text.

src/Diwink.Extensions.EntityFrameworkCore/Exceptions/PartialMutationNotAllowedException.cs (1)

11-20: Enforce non-empty unsupportedBranch input.

Line 19 persists unsupportedBranch directly; null/blank values degrade the aggregated rejection message.

Proposed fix

     public PartialMutationNotAllowedException(
         string relationshipPath,
         string unsupportedBranch)
         : base(
             $"Graph operation rejected: unsupported mutation detected at '{unsupportedBranch}'. " +
             "The entire operation was rejected — partial mutation is not allowed.",
             relationshipPath)
     {
+        ArgumentException.ThrowIfNullOrWhiteSpace(unsupportedBranch);
         UnsupportedBranch = unsupportedBranch;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore/Exceptions/PartialMutationNotAllowedException.cs`
around lines 11 - 20, The constructor PartialMutationNotAllowedException should
validate that the incoming unsupportedBranch is not null/empty/whitespace before
assigning; update the PartialMutationNotAllowedException(string
relationshipPath, string unsupportedBranch) constructor to check
string.IsNullOrWhiteSpace(unsupportedBranch) and throw an ArgumentException or
ArgumentNullException with a clear parameter name if invalid, then assign
UnsupportedBranch only after validation so the aggregated rejection message
never contains an empty value.

src/Diwink.Extensions.EntityFrameworkCore/Exceptions/UnsupportedRelationshipPatternException.cs (1)

10-19: Guard patternIdentifier before storing it as contract data.

Line 18 stores patternIdentifier without validation; empty values make rejection diagnostics ambiguous.

Proposed fix

     public UnsupportedRelationshipPatternException(
         string relationshipPath,
         string patternIdentifier)
         : base(
             $"Unsupported relationship pattern '{patternIdentifier}' at '{relationshipPath}'. " +
             "See the v2 contract documentation for supported patterns.",
             relationshipPath)
     {
+        ArgumentException.ThrowIfNullOrWhiteSpace(patternIdentifier);
         PatternIdentifier = patternIdentifier;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore/Exceptions/UnsupportedRelationshipPatternException.cs`
around lines 10 - 19, The constructor for
UnsupportedRelationshipPatternException stores patternIdentifier directly into
the exception contract data without validation; add a guard in the
UnsupportedRelationshipPatternException constructor to validate
patternIdentifier (throw ArgumentNullException or ArgumentException for
null/empty/whitespace) before assigning PatternIdentifier and before any
contract-storage logic so consumers get a clear rejection diagnostic; update the
constructor that currently sets PatternIdentifier to perform this check and
throw a descriptive argument exception mentioning "patternIdentifier" when
invalid.

src/Diwink.Extensions.EntityFrameworkCore/RelationshipStrategies/OneToOneOwnershipResolver.cs (1)

20-23: Add null guard before metadata dereference.

A defensive check avoids an ambiguous NRE at Line 22 if a null foreign key is ever passed.

♻️ Suggested hardening

 public static bool IsRequiredDependent(IForeignKey foreignKey)
 {
+    ArgumentNullException.ThrowIfNull(foreignKey);
     return foreignKey.IsRequired;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore/RelationshipStrategies/OneToOneOwnershipResolver.cs`
around lines 20 - 23, IsRequiredDependent currently dereferences
foreignKey.IsRequired without validation; add a null guard at the start of the
method (in the IsRequiredDependent method) to validate the foreignKey parameter
and throw an ArgumentNullException (include the parameter name) if it's null so
the method fails fast with an explicit error instead of producing an ambiguous
NRE.

src/Diwink.Extensions.EntityFrameworkCore/RelationshipStrategies/RequiredOneToOneStrategy.cs (1)

17-23: Add defensive argument guards at the strategy boundary.

If upstream orchestration ever passes null, failure at Line 19 will be a less-informative NRE. Guarding inputs here improves diagnostics.

♻️ Suggested hardening

 public static void RemoveDependent(DbContext context, ReferenceEntry existingNavigation)
 {
+    ArgumentNullException.ThrowIfNull(context);
+    ArgumentNullException.ThrowIfNull(existingNavigation);
+
     var existingValue = existingNavigation.CurrentValue;
     if (existingValue is null)
         return;

     context.Remove(existingValue);
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore/RelationshipStrategies/RequiredOneToOneStrategy.cs`
around lines 17 - 23, RequiredOneToOneStrategy.RemoveDependent currently
dereferences parameters without validation; add defensive argument guards at the
strategy boundary by validating both context and existingNavigation are not null
and throwing ArgumentNullException with appropriate param names (use
nameof(context) and nameof(existingNavigation)) at the top of the
RemoveDependent method before accessing existingNavigation.CurrentValue; keep
the existing null-check for existingNavigation.CurrentValue as-is.

src/Diwink.Extensions.EntityFrameworkCore.Tests.Integration/Contracts/ManyToMany/ManyToManySafetyContractTests.cs (1)

13-14: Redundant [Collection] attribute.

The [Collection(IntegrationTestCollection.Name)] attribute is already applied to IntegrationTestBase (Line 10 in that file). This attribute on the derived class is redundant.

Proposed fix

-[Collection(IntegrationTestCollection.Name)]
 public class ManyToManySafetyContractTests : IntegrationTestBase

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore.Tests.Integration/Contracts/ManyToMany/ManyToManySafetyContractTests.cs`
around lines 13 - 14, The [Collection(IntegrationTestCollection.Name)] attribute
on the ManyToManySafetyContractTests class is redundant because
IntegrationTestBase already declares it; remove the attribute from the
ManyToManySafetyContractTests class declaration so the class simply reads
"public class ManyToManySafetyContractTests : IntegrationTestBase", and run
tests to confirm no behavior change.

src/Diwink.Extensions.EntityFrameworkCore/RelationshipStrategies/PureManyToManyStrategy.cs (1)

48-75: Consider edge case: context.Find may return stale data if not already tracked.

When context.Find returns a database-loaded entity (Line 55), and you immediately call SetValues (Line 60), this works correctly. However, if the entity was already tracked with modifications from a previous operation within the same context lifetime, those modifications would be overwritten.

This is likely acceptable given the expected usage pattern (fresh context per operation), but worth noting for future maintenance.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore/RelationshipStrategies/PureManyToManyStrategy.cs`
around lines 48 - 75, context.Find can return an already-tracked entity whose
in-memory changes would be clobbered by the unconditional
context.Entry(knownEntity).CurrentValues.SetValues(updatedItem); modify the
logic inside the PureManyToManyStrategy flow to first inspect the tracked state
(via context.Entry(knownEntity).State) and only call SetValues when the entity
was not previously modified (e.g., State is Unchanged or Detached); otherwise
either merge selective properties or skip SetValues and just
AddToCollection(knownEntity) so you don’t overwrite local changes from the same
DbContext.

src/Diwink.Extensions.EntityFrameworkCore.Tests.Integration/Contracts/OneToOne/RequiredOneToOneContractTests.cs (1)

33-34: Unused variable policyVersion.

The policyVersion variable is captured but never used in the test. Either use it in an assertion or remove it.

Proposed fix

         existing.Policy.Should().NotBeNull("seed data includes a CoursePolicy");
-        var policyVersion = existing.Policy!.PolicyVersion;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore.Tests.Integration/Contracts/OneToOne/RequiredOneToOneContractTests.cs`
around lines 33 - 34, The local variable policyVersion (assigned from
existing.Policy!.PolicyVersion) is unused; either remove that assignment or
assert something with it. Locate RequiredOneToOneContractTests and the lines
referencing existing.Policy and policyVersion, then either delete the
policyVersion assignment, or replace it with an assertion using policyVersion
(for example assert the expected PolicyVersion from seed data against
existing.Policy!.PolicyVersion) so the variable is consumed.

src/Diwink.Extensions.EntityFrameworkCore.Tests.Unit/RelationshipSemantics/ManyToManyDiffStrategyTests.cs (1)

84-125: Single-context pattern may cause change tracker interference.

Unlike Detects_added_skip_navigation_entries, this test uses a single context for seed, act, and assert phases. While this works with InMemory provider, it doesn't test the same code paths as production (where detached graphs are compared against freshly-loaded tracked entities).

Also, Line 94: CatalogId = Guid.NewGuid() references a non-existent catalog. InMemory provider doesn't enforce FK constraints, but this could mask real issues.

For consistency, consider restructuring to match the multi-context pattern used in the first test.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore.Tests.Unit/RelationshipSemantics/ManyToManyDiffStrategyTests.cs`
around lines 84 - 125, The test Detects_removed_skip_navigation_entries uses a
single DbContext (CreateInMemoryContext) for arrange/act/assert which can let
the change tracker hide bugs; refactor it to follow the multi-context pattern
used in Detects_added_skip_navigation_entries by seeding data in one context
(ctx seed), then opening a fresh context to load existing via Include(c =>
c.Tags) into a tracked entity (existing), build an updated detached Course
object (updated) with the same CatalogId as the seeded course (do not call
CatalogId = Guid.NewGuid()), call InsertUpdateOrDeleteGraph(updated, existing)
on the fresh context, SaveChangesAsync, and finally verify results using a new
fresh context that Includes Tags so FK/catalog consistency is preserved and the
test exercises detached-vs-tracked logic.

src/Diwink.Extensions.EntityFrameworkCore.Tests.Integration/Infrastructure/DatabaseBootstrap.cs (1)

50-59: Consider adding timeout or retry for schema reset operations.

EnsureDeletedAsync and EnsureCreatedAsync can be slow or fail transiently when the SQL Server container is under load. For robustness in CI environments, consider wrapping with a command timeout or retry policy.

This is a nice-to-have for CI reliability; the current implementation is functionally correct.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore.Tests.Integration/Infrastructure/DatabaseBootstrap.cs`
around lines 50 - 59, Wrap ResetSchemaAsync's calls to
context.Database.EnsureDeletedAsync and EnsureCreatedAsync with a
retry-and-timeout strategy to handle transient SQL Server container load; for
example, acquire the DbContext from CreateContext(connectionString) and retry
the EnsureDeletedAsync/EnsureCreatedAsync sequence a few times with short delays
and per-call cancellation token timeouts, or use a Polly retry policy that
retries on transient exceptions and enforces a per-attempt timeout—make sure to
dispose the context between retries and surface a clear error if retries are
exhausted.

src/Diwink.Extensions.EntityFrameworkCore/RelationshipStrategies/PayloadManyToManyStrategy.cs (1)

68-77: The reflection-based approach works correctly for standard collection types; consider aligning with PureManyToManyStrategy for consistency.

GetMethod("Add") successfully retrieves the method for List<T>, HashSet<T>, and other standard collection types—each has exactly one public Add method. The current implementation is not broken.

However, for consistency and maintainability, adopt the same interface-based lookup pattern used in PureManyToManyStrategy.ExecuteCollectionOperation: check for IList first, then search GetInterfaces() for ICollection<> and retrieve the method from the interface rather than the concrete type.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore/RelationshipStrategies/PayloadManyToManyStrategy.cs`
around lines 68 - 77, The AddToCollection method currently uses concrete-type
reflection (GetMethod("Add")); change it to follow the pattern in
PureManyToManyStrategy.ExecuteCollectionOperation: if navigation.CurrentValue
implements IList use that directly, otherwise find an implemented generic
ICollection<> via navigation.CurrentValue.GetType().GetInterfaces(), get the Add
method from the ICollection<> interface (not the concrete type), and invoke that
method with the item; keep the existing InvalidOperationException messages but
replace the GetMethod("Add") lookup and invocation in AddToCollection with the
IList/ICollection<> detection and interface-based Add invocation.

src/Diwink.Extensions.EntityFrameworkCore/GraphUpdate/EntityKeyHelper.cs (1)

29-42: Consider consistent error handling between tracked and detached key extraction.

The tracked overload (GetKeyValues(EntityEntry)) throws InvalidOperationException when the entity lacks a primary key, but the detached overload silently returns an empty array. This inconsistency could mask configuration errors.

Consider throwing for consistency

     public static object[] GetKeyValues(DbContext context, object entity)
     {
         var entityType = context.Model.FindEntityType(entity.GetType());
         if (entityType is null)
-            return [];
+            throw new InvalidOperationException(
+                $"Entity type '{entity.GetType().Name}' is not part of the model.");

         var primaryKey = entityType.FindPrimaryKey();
         if (primaryKey is null)
-            return [];
+            throw new InvalidOperationException(
+                $"Entity '{entityType.ClrType.Name}' does not define a primary key.");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Diwink.Extensions.EntityFrameworkCore/GraphUpdate/EntityKeyHelper.cs`
around lines 29 - 42, GetKeyValues(DbContext, object) currently returns an empty
array when the entity type or primary key is missing; change it to mirror the
tracked overload GetKeyValues(EntityEntry) by throwing an
InvalidOperationException instead of returning [] when
context.Model.FindEntityType(entity.GetType()) or entityType.FindPrimaryKey() is
null, and keep the rest of the method (calling GetRequiredDetachedKeyValue and
ToArray) the same so error handling is consistent between tracked and detached
key extraction.

src/Diwink.Extensions.EntityFrameworkCore.Tests.Integration/Contracts/ManyToMany/PureManyToManyContractTests.cs (1)

145-179: Verify intended behavior: updating shared entity through association.

This test validates that updating a TopicTag label through the association propagates the change. However, if Tag1 is linked to multiple courses, this update affects all of them—which may or may not be the intended behavior.

Consider documenting this behavior explicitly in either the test name or a comment, as it represents a potentially surprising side effect for API consumers.

Consider clarifying the test intent

     [Fact]
-    public async Task Update_existing_related_entity_through_association_updates_entity_state()
+    public async Task Update_existing_related_entity_through_association_updates_entity_globally()
     {
+        // Note: This updates the shared TopicTag entity, affecting all courses linked to it.
+        // This is expected behavior for many-to-many relationships where the related entity
+        // is a shared principal, not an owned dependent.
+
         // Arrange

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore.Tests.Integration/Contracts/ManyToMany/PureManyToManyContractTests.cs`
around lines 145 - 179, The test
Update_existing_related_entity_through_association_updates_entity_state
currently updates a shared TopicTag (TopicTag with Id SeedData.Tag1Id) via the
association which will mutate that tag globally across all courses; update the
test to make the intent explicit by either renaming the test to indicate
shared-entity propagation (e.g., ..._updates_shared_entity_globally) or add a
concise comment above the arrange/assert sections explaining that updating a
TopicTag through InsertUpdateOrDeleteGraph will update the underlying TopicTag
for all courses that reference it, and optionally add an extra assert that
verifies another course referencing SeedData.Tag1Id also sees the changed Label
to codify the observed behavior.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

.github/workflows/dotnet-core.yml (1)

14-66: Optional: add workflow concurrency to reduce overlapping publish attempts.

This is not a blocker, but concurrency control helps avoid duplicate runs on rapid successive pushes.

Suggested refinement

 name: .NET CI
+
+concurrency:
+  group: dotnet-ci-${{ github.ref }}
+  cancel-in-progress: true

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/dotnet-core.yml around lines 14 - 66, Add GitHub Actions
concurrency to the publish job to prevent overlapping publish runs: inside the
existing publish job (named "publish") add a concurrency block such as
"concurrency: { group: 'publish-${{ github.ref }}', cancel-in-progress: true }"
(or similar group key to suit your branch/tag naming) so new publish runs cancel
in-progress ones and avoid duplicate NuGet pushes.

src/Diwink.Extensions.EntityFrameworkCore/GraphUpdate/OperationGuard.cs (1)

35-38: XML documentation is misleading for the multiple-errors case.

The doc states "Throws the first recorded error if any exist" but when multiple errors are recorded, the method throws a PartialMutationNotAllowedException (a different exception type), not the first error.

📝 Suggested documentation fix

     /// <summary>
-    /// Throws the first recorded error if any exist, enforcing all-or-nothing
-    /// rejection before mutations are applied to the change tracker.
+    /// Throws if any errors were recorded, enforcing all-or-nothing rejection
+    /// before mutations are applied to the change tracker. A single error is
+    /// rethrown directly; multiple errors are wrapped in a
+    /// <see cref="PartialMutationNotAllowedException"/>.
     /// </summary>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Diwink.Extensions.EntityFrameworkCore/GraphUpdate/OperationGuard.cs`
around lines 35 - 38, Update the XML doc on OperationGuard.ThrowIfErrors to
accurately describe its behavior: explain that if one error is recorded the
method rethrows that single recorded exception, but if multiple errors are
recorded it throws a PartialMutationNotAllowedException containing aggregated
error information; reference the OperationGuard class and the
PartialMutationNotAllowedException type in the summary so the documentation
matches the implemented behavior.

src/Diwink.Extensions.EntityFrameworkCore.Tests.Unit/RelationshipSemantics/ManyToManyDiffStrategyTests.cs (2)

199-249: Use a fresh verify context for assertion consistency.

This test asserts using the same context used for arrange/act, while other tests use fresh verification contexts. Aligning this test to the same pattern reduces state-carryover risk and keeps semantics consistent across the suite.

Proposed structure alignment

     public async Task Detects_added_payload_association_entities()
     {
-        // Arrange
-        await using var ctx = CreateInMemoryContext();
+        var dbName = Guid.NewGuid().ToString();
+
+        // Arrange
+        await using var ctx = CreateInMemoryContext(dbName);
         var mentor = new Mentor { Id = Guid.NewGuid(), DisplayName = "M1", Status = "Active" };
         var course = new Course
         {
@@
         ctx.InsertUpdateOrDeleteGraph(updated, existing);
         await ctx.SaveChangesAsync();

         // Assert
-        var assignments = await ctx.CourseMentorAssignments
+        await using var verifyCtx = CreateInMemoryContext(dbName);
+        var assignments = await verifyCtx.CourseMentorAssignments
             .Where(a => a.CourseId == course.Id)
             .ToListAsync();
         assignments.Should().HaveCount(1);
         assignments[0].Role.Should().Be("Lead");
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore.Tests.Unit/RelationshipSemantics/ManyToManyDiffStrategyTests.cs`
around lines 199 - 249, The test Detects_added_payload_association_entities uses
the same DbContext (ctx) for Arrange/Act and Assert which can leak tracked
state; change the assertion phase to use a fresh verification context created
via CreateInMemoryContext() so you query persisted state cleanly. Specifically,
after await ctx.SaveChangesAsync() and before assertions, dispose/stop using the
original ctx and create a new var verifyCtx = CreateInMemoryContext(); then
query verifyCtx.CourseMentorAssignments (instead of ctx.CourseMentorAssignments)
to assert the created CourseMentorAssignment; keep usage of
InsertUpdateOrDeleteGraph and existing test data unchanged.

---

74-81: Strengthen assertion to validate membership, not only count.

Line 80 only checks cardinality; it can pass with the wrong tag identities. Assert the resulting tag IDs as well.

Proposed assertion hardening

             var result = await verifyCtx.Courses
                 .Include(c => c.Tags)
                 .FirstAsync(c => c.Id == courseId);
-            result.Tags.Should().HaveCount(2);
+            result.Tags.Should().HaveCount(2);
+            result.Tags.Select(t => t.Id).Should().BeEquivalentTo([tag1Id, tag2Id]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore.Tests.Unit/RelationshipSemantics/ManyToManyDiffStrategyTests.cs`
around lines 74 - 81, The test currently only asserts
result.Tags.Should().HaveCount(2) which can hide wrong identities; update the
assertion in ManyToManyDiffStrategyTests (inside the verify block using
CreateInMemoryContext and verifyCtx.Courses.Include(c =>
c.Tags).FirstAsync(...)) to assert the tag identities too — e.g., collect
result.Tags.Select(t => t.Id) and assert it matches the expected tag id set (use
FluentAssertions' BeEquivalentTo or Should().Contain/OnlyContain with the known
tagId variables used earlier in the test, e.g., expectedTagId1/expectedTagId2)
so the test verifies membership not just count.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/Diwink.Extensions.EntityFrameworkCore.Tests.Integration/Infrastructure/DatabaseBootstrap.cs (1)

121-125: Consider narrowing the retryable exception scope.

InvalidOperationException is quite broad and could mask genuine programming errors. For SQL Server connectivity issues, SqlException typically covers transient failures. Consider whether InvalidOperationException is truly needed here, or if specific subtypes/messages should be checked.

That said, for test infrastructure against containerized SQL Server, this broad catch may be pragmatic given connection pool edge cases during container startup.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore.Tests.Integration/Infrastructure/DatabaseBootstrap.cs`
around lines 121 - 125, The IsRetryableSchemaException method currently treats
every InvalidOperationException as retryable which is too broad; narrow it by
removing the blanket InvalidOperationException check and instead only treat
InvalidOperationException as retryable when it matches known transient scenarios
(e.g., specific message/errno/inner-exception patterns caused by SQL
connectivity or pool initialization), or drop it entirely and rely on
SqlException/TimeoutException plus the cancellation/OperationCanceledException
branch; update IsRetryableSchemaException to check exception is SqlException or
TimeoutException or (cancellationSource.IsCancellationRequested && exception is
OperationCanceledException), and if you must keep InvalidOperationException, add
targeted checks for known transient messages/inner-exceptions in that function
(use the exception.Message or exception.InnerException type to identify
transient pool/startup errors).

src/Diwink.Extensions.EntityFrameworkCore/RelationshipStrategies/PureManyToManyStrategy.cs (1)

55-83: Consider batching context.Find calls for large collections.

Each unlinked entity in updatedItems triggers a context.Find() call (Line 63), which may query the database if the entity isn't tracked. For collections with many new links, this could cause N+1 query behavior.

For typical use cases with small collections, this is acceptable. If performance becomes a concern with larger graphs, consider pre-loading candidate entities in a single query.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore/RelationshipStrategies/PureManyToManyStrategy.cs`
around lines 55 - 83, The per-item context.Find call in PureManyToManyStrategy
(see usage of context.Find, updatedItem and updatedKeys) can cause N+1 queries
for large updatedItems collections; modify the logic to batch-load candidate
entities before the loop by collecting all primary key values for the target
entity type, querying the DbSet for those keys in a single query to build a
lookup (dictionary by key), then inside the loop use the lookup to resolve
knownEntity and only fall back to context.Find or Add when a key is missing;
keep ApplyValuesIfNotModified and AddToCollection usage but replace the repeated
context.Find calls with the preloaded lookup to avoid many DB roundtrips.

src/Diwink.Extensions.EntityFrameworkCore.Tests.Integration/Infrastructure/SeedDataStructureTests.cs (1)

53-57: Rename this test or assert the “catalog prefix” invariant.

The method name/comment says we're checking a shared catalog prefix, but the assertion only verifies that the two IDs differ. That mismatch makes the test intent harder to trust when it fails.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Diwink.Extensions.EntityFrameworkCore.Tests.Integration/Infrastructure/SeedDataStructureTests.cs`
around lines 53 - 57, The test Course1Id_and_Course2Id_share_same_catalog_prefix
currently only asserts
SeedData.Course1Id.Should().NotBe(SeedData.Course2Id);—either rename the test to
reflect that it only verifies inequality, or add an assertion that checks the
catalog prefix invariant: extract the catalog prefix from SeedData.Course1Id and
SeedData.Course2Id (using the same parsing logic used in your ID type) and
assert they are equal while still asserting the IDs are not equal; update the
test name if you choose to keep only the inequality check (e.g.,
CourseIds_are_distinct) or keep the existing name and add the prefix equality
assertion.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes